Conversation
crashes when using test stand astig removal tool
|
I know I need to review. I keep this one for later. |
atsju
left a comment
atsju
left a comment
There was a problem hiding this comment.
Reviewed. This is valid fix. Thank you.
I do not approve as there are 2 small comments to manage
zernikepolar.cpp
Outdated
| } | ||
|
|
||
| if(nbTerms > 35) | ||
| if(nbTerms > 35) // shouldn't this be 36? |
There was a problem hiding this comment.
yes correct. and it should also be modified for 8 and 24.
There was a problem hiding this comment.
oops. Just checked in the fix.
There was a problem hiding this comment.
Actually it was just suboptimal but could not cause issue. Good to have corrected the code though
Co-authored-by: Julien Staub <atsju2@yahoo.fr>
|
I think I should get rid of most of the logging as well. |
they are at "trace" level. It's meant for exactly that. I would not remove them. Default users shall not log at trace level unless they have problems |
oh. I already got rid of many of them. Don't they slow down execution slightly? Even if log is not on trace mode? |
I don't know how much. Possibly yes. |
2 participants
Fixes #246 a bug that occurs in "test stand astig removal"